Skip to content

Add SwingColorAlphaWidget #48

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Oct 7, 2020
Merged

Add SwingColorAlphaWidget #48

merged 1 commit into from
Oct 7, 2020

Conversation

frauzufall
Copy link
Member

@frauzufall frauzufall commented May 16, 2020

This PR makes it possible to use parameters of typeColorRGBA. Before, when using ColorRGBA, the SwingColorWidget would return null when changing the color because ColorRGB cannot be converted to ColorRGBA here.

I just quickly copied the SwingColorWidget and replaced ColorRGB with ColorRGBA which makes it work (and also possible to assign colors with alpha value). Let me know if there is a cleaner solution and maybe a way to write a test.

frauzufall added a commit to juglab/LabelEditor that referenced this pull request May 16, 2020
Copy link
Member

@imagejan imagejan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just added a few comments, didn't do extensive testing.

What's your use case for this widget?

@frauzufall
Copy link
Member Author

frauzufall commented May 17, 2020

Thanks for looking at it @imagejan! Here's my use case, it's the LabelEditor where I already copied the widget into in order to use it for an interactive command to change the default colors of any open labeling model:
Screencast-2020-05-17-13_50_38

@frauzufall
Copy link
Member Author

The button icon was displaying the RGBA value as RGB (see screencast above) - I fixed that now.

@frauzufall
Copy link
Member Author

frauzufall commented May 18, 2020

I removed duplicate static methods which were copied into SwingColorAlphaWidget and referenced the ones from SwingColorWidget.

I noticed that the tab reordering of the color chooser is not working - the code comment says it would switch from Swatches, HSB, RGB to HSB, RGB, Swatches, but for me it doesn't, see screencast above. Is it working for anyone else? I don't have enough incentive to dig deeper, I'd be fine with the default ordering and just remove this part of the code.. @ctrueden you are listed as maintainer, do you have an opinion? Otherwise I'd just merge this if you don't object.

@ctrueden
Copy link
Member

@ctrueden you are listed as maintainer, do you have an opinion?

Would it be possible to have a single SwingColorWidget that supports both RGB and RGBA cases? This would reduce code duplication, no?

@frauzufall
Copy link
Member Author

@ctrueden I pushed a version where SwingColorWidget supports both. It does check the type via ColorRGBA.class.isAssignableFrom(get().getValue().getClass())) and I thought that's not ideal.. But this of course avoids more code duplication. It works for me.

@imagejan
Copy link
Member

@frauzufall wrote:

It does check the type via ColorRGBA.class.isAssignableFrom(get().getValue().getClass())) and I thought that's not ideal..

You could also use SciJava's Types utility class:

Types.isAssignable(get().getValue().getClass(), ColorRGBA.class)

or

Types.isInstance(get().getValue(), ColorRGBA.class)

... but I don't know if that makes any difference.

* while the SwingColorWidget was already able to be used for ColorRGBA, is was previously not able to actually propagate the alpha choice of the user
* added a widget demo
@frauzufall frauzufall merged commit e68111f into master Oct 7, 2020
@frauzufall frauzufall deleted the widget-rgba branch October 7, 2020 14:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants